Skip to content

Correct check when slicing non-monotonic datetime indexes #54002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Jul 5, 2023

The intention of #37819 was to deprecate (removed in #49607) the special case behaviour of non-monotonic datetime indexes, so that if either slice bound is not in the index, a KeyError is raised.

However, the check only fired correctly for the case where the lower bound was not in the index and either the upper bound was None or it was also not in the index.

Correct the logic here and adapt the one test that exercises this behaviour.

Closes #53983.

The intention of pandas-dev#37819 was to deprecate (removed in pandas-dev#49607) the
special case behaviour of non-monotonic datetime indexes, so that if
either slice bound is not in the index, a KeyError is raised.

However, the check only fired correctly for the case where the lower
bound was not in the index and either the upper bound was None or it
was _also_ not in the index.

Correct the logic here and adapt the one test that exercises this
behaviour.

Closes pandas-dev#53983.
@wence-
Copy link
Contributor Author

wence- commented Jul 5, 2023

cc @phofl.

This might need to go through a deprecation cycle since although the intention was to warn for this behaviour through 1.5 and remove in 2, users who only had upper slice bounds that were missing would not have seen that warning.

@wence-
Copy link
Contributor Author

wence- commented Jul 18, 2023

Any thoughts here?

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be okay to note as a "bug fix" in 2.1 since the original behavior was deprecated but not thoroughly enforced.

@mroeschke mroeschke added Datetime Datetime data dtype Indexing Related to indexing on series/frames, not to indexes themselves labels Jul 18, 2023
@wence-
Copy link
Contributor Author

wence- commented Jul 19, 2023

I think this would be okay to note as a "bug fix" in 2.1 since the original behavior was deprecated but not thoroughly enforced.

Thanks, added a bugfix entry.

@mroeschke mroeschke added this to the 2.1 milestone Jul 19, 2023
@mroeschke mroeschke merged commit 0fff817 into pandas-dev:main Jul 19, 2023
@mroeschke
Copy link
Member

Awesome, thanks @wence-

@wence- wence- deleted the wence/fix/issue-53983 branch July 19, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Partial slicing of unordered datetimeindex inconsistent between providing start and end points
2 participants